Skip to content

quickstart: Fix silent failure when .gitignore existsThis fixes issue… - #1583

Open
harshil2424 wants to merge 1 commit into
airspeed-velocity:mainfrom
harshil2424:fix-issue-1582
Open

quickstart: Fix silent failure when .gitignore existsThis fixes issue…#1583
harshil2424 wants to merge 1 commit into
airspeed-velocity:mainfrom
harshil2424:fix-issue-1582

Conversation

@harshil2424

Copy link
Copy Markdown

#1582 where asv quickstart aborted if a .gitignorewas present. The command now appends ASV-specific patterns to the existingfile instead of treating it as a template conflict.

…irspeed-velocity#1582 where \�sv quickstart\ aborted if a \.gitignore\was present. The command now appends ASV-specific patterns to the existingfile instead of treating it as a template conflict.
@harshil2424

Copy link
Copy Markdown
Author

Fixes #1582

@HaoZeke HaoZeke left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IMO this is a rather large change to the existing behavior. My preferred fix would be a noisier / clearer error message instead of changing the way asv works.

@mtryan83

Copy link
Copy Markdown

I followed roughly the same steps in #1582 and wound up here. The problem I see with just changing the error message is the user then needs to

  1. create/find their own asv.conf.json template file
    • the template isn't provided in the documentation, but all the fields are listed
    • it wasn't entirely clear where the config file went (top-level? benchmarks folder?), but that might have been just a misreading/misunderstanding on my part
  2. Create a benchmarks folder and populate it with benchmarks.py and empty __init__.py.
  3. Check/create their .gitignore to verify they're ignoring the same things (slightly out-of-date, standard python .gitignore with a typo).

Note that this bug will also occur if the benchmarks folder already exists, even if benchmarks.py/__init__.py don't.

Suggestions

  1. The "template content already exists" message should be expanded to include what template content already exists.
  2. Add link(s) to missing template files/contents somewhere
    • Since the contents of benchmarks.py immediately follow in the documentation, and they'll need to be overwritten anyway, this is less of an issue
  3. Instead of modifying the .gitignore file, copy the missing files and leave the already existing files alone.

I think (1) is the minimum to fix the issue, but (2) and/or (3) would be very helpful.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants